Remove comparisons of this with NULL. Calling functions on objects that are NULL is undefined behavior. Hence, new versions of clang assume that `this` is never NULL, warn on comparisons of `this` with NULL, and optimize the comparison away. There were three comparisons of `this` with NULL in re2: 1. CharClass::Delete(). There are only two callers of this method, and as far as I can tell `this` can't be NULL there, so just delete that check. 2. Prefilter::DebugString(). This too has two callers: Prefilter::Info::ToString(), which already checks for NULL before calling, and DebugString() itself. Since several places check Prefilters for NULL, add explicit checks before calling here. 3. Prefilter::Info::ToString(). This has three callers. In 2 cases this can't be NULL. In the third case it could conceivably be NULL if op() is kRegexpConcat and nchild_args is 0, so add an explicit check here. Fixes https://code.google.com/p/re2/issues/detail?id=115 LGTM=rsc R=rsc CC=re2-dev https://codereview.appspot.com/107100043
diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 2a694bc..1a1c848 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS
@@ -31,6 +31,7 @@ Dmitriy Vyukov <dvyukov@google.com> John Millikin <jmillikin@gmail.com> Mike Nazarewicz <mpn@google.com> +Nico Weber <thakis@chromium.org> Pawel Hajdan <phajdan.jr@gmail.com> Rob Pike <r@google.com> Russ Cox <rsc@swtch.com>
diff --git a/re2/prefilter.cc b/re2/prefilter.cc index 1b12458..f192f9b 100644 --- a/re2/prefilter.cc +++ b/re2/prefilter.cc
@@ -265,14 +265,6 @@ // Format a Info in string form. string Prefilter::Info::ToString() { - if (this == NULL) { - // Sometimes when iterating on children of a node, - // some children might have NULL Info. Adding - // the check here for NULL to take care of cases where - // the caller is not checking. - return ""; - } - if (is_exact_) { int n = 0; string s; @@ -640,7 +632,7 @@ if (Trace) { VLOG(0) << "BuildInfo " << re->ToString() - << ": " << info->ToString(); + << ": " << (info ? info->ToString() : ""); } return info; @@ -665,9 +657,6 @@ } string Prefilter::DebugString() const { - if (this == NULL) - return "<nil>"; - switch (op_) { default: LOG(DFATAL) << "Bad op in Prefilter::DebugString: " << op_; @@ -683,7 +672,8 @@ for (int i = 0; i < subs_->size(); i++) { if (i > 0) s += " "; - s += (*subs_)[i]->DebugString(); + Prefilter* sub = (*subs_)[i]; + s += sub ? sub->DebugString() : "<nil>"; } return s; } @@ -692,7 +682,8 @@ for (int i = 0; i < subs_->size(); i++) { if (i > 0) s += "|"; - s += (*subs_)[i]->DebugString(); + Prefilter* sub = (*subs_)[i]; + s += sub ? sub->DebugString() : "<nil>"; } s += ")"; return s; diff --git a/re2/regexp.cc b/re2/regexp.cc index a74ceec..ed4c3a0 100644 --- a/re2/regexp.cc +++ b/re2/regexp.cc
@@ -873,8 +873,6 @@ } void CharClass::Delete() { - if (this == NULL) - return; uint8 *data = reinterpret_cast<uint8*>(this); delete[] data; } 